Skip to content

fix(cli): parse os package publish manifest ids through PackageSchema.manifestId - #16889

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-16520-manifest-id-parse-through-schema
Sep 8, 2026
Merged

fix(cli): parse os package publish manifest ids through PackageSchema.manifestId#16889
os-project-manager merged 1 commit into
mainfrom
claude/issue-16520-manifest-id-parse-through-schema

Conversation

@claude

@claude claude Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #16520

Clause-②: no

os package publish carried its own MANIFEST_ID_RE and validated --manifest-id (and the derived id) against it, while the contract this repo declares for the column it publishes into is PackageSchema.manifestId in packages/spec/src/cloud/package.zod.ts. The local copy was looser on every axis, so the preflight admitted what the control plane refuses — and its error text, when it did fire, named a contract that does not exist.

The remedy is the one the maintainer's ruling on cloud#1932 already settled — 「cloud#1932 不普查,直接按协议修改。」 under 「本项目以协议为基准。」 — import, never transcription. CreatePackageRequestSchema in the same spec file already reaches for PackageSchema.shape.manifestId the same way (verified by symbol at current head, not by line number).

验收 — evidence against each item

1. MANIFEST_ID_RE deleted entirely; both paths routed through the schema.
The constant is gone from the file (git grep MANIFEST_ID_RE packages/cli returns nothing). Both sites now ask the imported schema:

  • derive path — deriveManifestId adopts artifact.manifest.id only when isManifestId(explicit), which is PackageSchema.shape.manifestId.safeParse(...).success;
  • explicit path — run() parses the supplied or derived id through the same schema before any network call.

The derive path's extra explicit.includes('.') condition is removed, not preserved: the schema's pattern requires at least two segments, so a dotless id can never parse and the condition is subsumed. Dropping it narrows nothing and widens nothing.

2. deriveManifestId closed. It previously trusted artifact.manifest.id through the same loose test, so com.acme.repair_desk and friends were forwarded unchanged. ManifestSchema.id is a bare z.string(), so an artifact may carry any shape at all; the deriver now adopts it only when the control plane would accept it, and otherwise falls through to the existing local.SLUG fallback.

3. The producer side — handled by refusing, deliberately not by normalising. slugify lowercases, collapses non-alphanumeric runs to hyphens, trims and truncates, with no letter-first rule, so a manifest named 2024 App derives local.2024-app — digit-first, and rejected by the schema. That id is now refused before any network call, with a message that names where it came from and how to set one (--manifest-id, manifestId in objectstack.manifest.json, or manifest.id in the config).

It is not normalised into some other id, and that is a decision rather than an omission: manifestId is declared immutable once set ("renaming a package requires creating a new package"), so a normaliser mints a permanent, globally unique identifier the author never wrote and cannot rename afterwards. "Prefer failing to falling back" (AGENTS.md → Route & surface ownership §3) and Prime Directive #12's "reject it at authoring/publish so the error surfaces loudly" both point the same way. Nothing is lost by refusing: that publish does not work today either — it fails one round trip later, with a worse message. cloud PR #2032's manifestIdSegment is named in the card as one worked normalisation; it could not be read from this seat (see the note at the end) and is not what this PR does.

4. Error text quoted from the schema. The refusal is built from the schema's own invalid_format issue message plus its .describe(); no second description of the pattern is written anywhere in the command. The retired sentence is what was lying: a user actually stopped by the CLI read a-z0-9._-, "fixed" their id to something like com.acme.repair_desk — accepted locally, refused by the server. Following the error message led to a second error. A test asserts the new text contains the schema's own issue message and its description, and a sibling test asserts the string a-z0-9._- no longer appears in the output.

5. Per-path assertions, per shape. packages/cli/test/package-publish-manifest-id.test.ts asserts all six shapes on both paths:

shape why it was admitted explicit path derive path
crm single segment refused (exit 1, zero fetch calls) not adopted
com.acme.repair_desk underscore refused not adopted
COM.ACME.CRM upper case refused not adopted
9foo.bar digit-first segment refused not adopted
com..acme empty segment refused not adopted
com.acme. trailing dot refused not adopted

Triage's correction is carried into the suite as data rather than as prose: RELAXATIONS records, per shape, whether the derive path admitted it before the fix — five did, crm did not, because of that dot condition. The two ablations below reproduce that asymmetry mechanically.

On the derive path "refuses" means the illegal manifest.id is not adopted: the deriver falls through to local.SLUG (its pre-existing behaviour for any id it will not take), and each case asserts both halves — the deriver returns { id: 'local.acme-crm', source: 'artifact-manifest-name' }, and end to end the manifest_id that reaches the wire is neither the illegal shape nor anything the schema rejects.

6. Negative control — mandatory, and present on both paths. com.acme.crm still publishes, bytes unchanged: on the explicit path calls[0].body.manifest_id is exactly com.acme.crm, and on the derive path deriveManifestId returns exactly { id: 'com.acme.crm', source: 'artifact-manifest-id' } and the same value reaches the wire. A second control covers the derived-but-legal case (Acme CRM publishes as local.acme-crm).

A re-transcription of the schema's regex would turn every refusal above green while reproducing this card's cause exactly, so the suite also carries a source pin: no fully anchored regex literal in publish.ts may match com.acme.crm — any local rule about manifest ids must, which is what makes it a rule about manifest ids. The pin runs with its own positive control beside it (the retired rule as text, and a transcription of the schema's own pattern — the scanner finds both), and additionally asserts that the source still contains PackageSchema.shape.manifestId, so the absence is a fix and not a deletion.

7. Clause-② re-declared from the delivered diff: no. The diff narrows a local preflight's accept set back onto the contract this repo declares. No id that is rejected today becomes valid: the deriver's fallback behaviour is unchanged, no normaliser was added, and the only condition removed (includes('.')) is subsumed by the schema. The single behaviour change in the other direction is that ids the server was going to reject are now refused locally.

8. Out of scope, untouched: cloud's three producers (cloud PR #2032) and create_package's REVERSE_DOMAIN_RE.

Reverse verification

Both legs were run from the committed implementation, each proving the mutation reached disk (blob hash before/after plus an on-disk marker count) and each restored with git checkout HEAD -- PATH, verified by git diff HEAD and git status --porcelain both empty and the blob hash back to the HEAD blob. Baseline: 22 passed.

ablation what was mutated result
derive path isManifestId body replaced by the retired rule plus the call site's dot condition 9 failed / 13 passed — the five shapes the loose rule admitted go red; crm stays green, which is exactly triage's asymmetry; the source pin fires
explicit path explainManifestId decided by the retired rule, returning the retired sentence 9 failed / 13 passed — all six shapes go red on this path, plus the schema-quoting test and the producer test; the source pin fires

One test stayed green under the explicit-path ablation and that is correct rather than a gap: "no longer states the contract the CLI invented" asserts the text, and under that mutation the command does not refuse com.acme.repair_desk at all, so nothing is printed. Its sibling in the same block — which asserts the schema's own issue message is present — is one of the nine that went red.

Tests and gates

Measured at 8756045d (the final commit) in a worktree branched from c930f8597. Heavy runs went through scripts/pm/os-verify-lock.sh; the box is shared, so the wall-clock figures below are shared-box seconds.

  • pnpm --filter @objectstack/cli exec vitest run --project unit187 files passed, 2577 passed | 6 expected fail (2583), exit 0. The integration tier is declared to CI: this diff touches no spawn entry point, no test/helpers/serve-process.ts and no driver/kernel boot path, and the new file lands in unit under packages/cli/vitest-tiers.ts (test/vitest-tiers-partition.test.ts passes).
  • pnpm --filter @objectstack/cli typecheck — exit 0. check:test-typecheck reports the ledger unchanged (3 files / 28 errors, shrink-only), and tsconfig.test.json includes test/**/*, so the new file is in a tsc program rather than merely on disk.
  • pnpm --filter '@objectstack/cli^...' build — exit 0 (the dependency closure, so every verdict above resolves workspace imports through real .d.ts rather than through an unbuilt world).
  • pnpm lint (eslint . --no-inline-config) — exit 0 over 6368 files, 0 findings, run whole rather than narrowed, at 8756045d. File count read from --format json; the population is eslint's own from eslint.config.mjs.
  • Gate families derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (it derives its own change set from the merge base): 59 commands, 57 run green. Exit codes were captured before any pipe.

Two are NOT MEASURED, reported as failed measurements rather than as passes:

  • pnpm check:dual-build-cjs-loadsPREREQUISITE NOT MET (exit 3): it reads built output and several packages outside this diff's closure have no dist/. It needs a whole-workspace build, which is CI's run.
  • pnpm check:type-check-debt — its prerequisite cleared after a rebuild, but the repo-wide --re-measure exceeds this container's ~10-minute foreground cap. Its sibling pnpm check:type-check-coverage ran green.

Three gates first reported NOTHING was measured against stale build state in this worktree (check:i18n, check:i18n-coverage, check:i18n-walk-parity) and are green after rebuilding @objectstack/metadata-core, @objectstack/cli and the four connector packages — the prescription in AGENTS.md → Multi-agent discipline §9, not a finding.

Two things this seat could not verify, stated as such

  • cloud PR feat(dogfood): multi-user harness + RLS cross-owner proof runner (#1994) #2032's current state is unverifiable from here. Triage asks the implementer to check it first. Measured three-way on this seat: objectui (positive control) returns refs, objectstack-ai/cloud answers could not read Username, and a repository that does not exist answers identically — so the probe cannot separate private from absent, and a REST probe returns 403 for readable repositories too (that 403 is the proxy, not GitHub). This does not block the change: the fix aligns the CLI to this repo's own declared schema, which is correct whatever cloud is doing. Cloud governs the urgency of the conflict window, not the direction of the fix. The card's "once it lands" is therefore not restated here as confirmed.
  • origin/main has moved since the branch point (9 commits, c930f8597a5d4e286b6). None of them touches packages/cli/src/commands/package/publish.ts or packages/spec/src/cloud/package.zod.ts; several touch other files in packages/cli. This branch was not merged with main, so the joint verdict is the merge queue's rebuilt generation.

Docs drift advisory

Re-derived rather than inherited. Angle-bracket placeholders below are spelled without their brackets (--manifest-id ID, local.SLUG) because this surface mutates tag-shaped fragments.

1. The advisory's own number, re-derived on a clean tree. The bot flagged a provenance defect in its own run ("that checkout carried uncommitted changes"). Re-run on the exact tree it names — a6204a5d87, the merge of head 8756045d1a into base 142c01c88e — in a detached worktree whose git status --porcelain was empty before the run, node scripts/docs-audit/affected-docs.mjs --json 142c01c88e reports computedOn.dirty: false and 12 docs (11 hand-written + 1 release-owned) across 11 anchors — the same rows, in the same order, with the same anchor attributions as the comment. On this PR the bot's number was exact; the provenance warning did not corrupt it here. The 11 anchors are the 9 symbols this diff adds or removes (MANIFEST_ID_RE among them, as a deletion), the literal com.acme.crm, and the os package publish command.

2. Every id-shaped literal in content/, parsed through the declaration. Swept all 406 files under content/ (not just the 12 rows) with five queries, each candidate parsed through PackageSchema.shape.manifestId imported from packages/spec/src/cloud/package.zod.ts at this tree — the same declaration the CLI now uses, not a transcription of it. Discrimination control on the parser: com.acme.crm accepted, com.acme.repair_desk rejected, crm rejected.

query occurrences distinct rejected by the schema
value following --manifest-id 3 3 2 — both non-values (see below)
quoted value on a line naming the key 11 4 1 — a prose fragment
every com.acme. token anywhere in content/ 15 3 0
every local. token anywhere in content/ 4 2 4 — all placeholders/prose
id-shaped token inside a fence showing a publish invocation 1 1 0

No real id literal in content/ is falsified. All seven "rejects" are placeholders or prose, not values a reader would copy: the flag metavariable ID in the options table, the word and from the sentence "falls back to --manifest-id and then to…", the prose fragment "local. + a slug of the artifact name", the placeholder local.SLUG, and two unrelated local. occurrences in kernel/cluster.mdx and references/automation/control-flow.mdx that are not manifest ids at all. Every concrete id the docs print in a publish context — com.acme.crm, com.acme.encryption, com.acme.pii — parses green. Control for the fence query: 2027 fences scanned corpus-wide, 5 contain a publish invocation, so the single row is a reading and not an empty scan.

3. content/docs/releases/v9.mdx — read, not edited. No fact in it is falsified by this diff. Stated explicitly because the answer is a negative: its os package publish passage covers publish-then-install and the --visibility default (private / org / marketplace, defaulting to org) and says nothing about manifest ids, their shape, what the command accepts or refuses, or its error text. Nothing to hand back for a docs-only PR or an issue from that page.

4. The retired error text appears nowhere in the docs. The sentence this PR deletes named a-z0-9._-; searching content/ for that contract, for Expected reverse-domain and for Invalid manifest-id returns 0 hits. Paired control sharing the vocabulary: reverse-domain / reverse domain returns 22 lines across 12 files, all read. So the zero is a reading, not a broken query. Of the ten os package publish pages, none states what the command accepts or refuses for a manifest id; the two rule-bearing rows are deployment/cli.mdx (the --manifest-id options row) and publish-and-preview.mdx (the two-POST sequence), and both state the precedenceartifact.manifest.id, else local. plus a slug — without stating the condition under which the fallback is taken. Both remain true.

One incompleteness, reported rather than fixed here (a docs-only change is not a rider on a code PR): deployment/cli.mdx's --manifest-id row presents local. plus a slug as an unconditional default. After this change a derived id the schema rejects — a manifest named 2024 App derives local.2024-app, digit-first — is refused before any network call, so the default can now fail loudly. The row does not say so. It was not falsified (it never claimed the fallback always succeeds), which is why this is a follow-up and not a correction.

5. The advisory's stated blind spot, answered by hand. The advisory says a page stating a rule by its inputs shares no identifier with the emitter that implements it, and this diff is emitter-side, so such a page cannot be listed on any run. Searched by hand for a page that states the id rule by its inputs: reverse-domain / reverse domain (22 lines, all read — 20 in generated references/** carrying the schema's own description, plus protocol/kernel/index.mdx and deployment/cli.mdx, both consistent), then package id / manifest id (74 further lines, all generated field descriptions with no shape rule in them). Exactly one page in content/ states an id rule by its inputs: plugins/development.mdx § Plugin ID Format, which gives a pattern allowing a single segment and calls out uppercase, spaces and underscores. That is the Studio plugin id, a different contract from PackageSchema.manifestId, and this diff does not touch it. It is also the control this hand search needed: the search does surface a by-inputs rule statement in the vocabulary that would have failed, so the zero for package-manifest-id rules is a reading rather than an empty grep.

验收备注

  • noted, not filed: packages/cli/src/commands/package/publish.ts keeps NAMESPACE_RE as a hand-copied mirror of ManifestSchema.shape.namespace / PackageSchema.shape.namespace. Unlike the manifest-id copy it is pinnedtest/package-publish-namespace.test.ts asserts the CLI rule and the spec schema agree value by value — so it is a maintained copy rather than a silent fork, and converting it is a different change from this card. Whoever next touches the publish preflight is the one who meets it.
  • filed as #16891: os package publish silently substitutes a derived id when the artifact declares a manifest.id the publish path will not use — the declared, authorable value is discarded with nothing said, and the substitute is a permanent immutable identifier. This PR makes that path reachable for MORE values (it narrows which manifest.id values are adopted), so it is named here rather than left implicit; whether the fallback should warn, refuse, or be enforced at the producer is a decision this seat has no ruling for.
  • noted, not filed: content/docs/deployment/cli.mdx's --manifest-id options row presents local. plus a slug as an unconditional default; after this change a derived id the schema rejects is refused before any network call, so the default can fail loudly and the row does not say so. Not falsified, so not corrected here — a docs-only change is not a rider on a code PR. Successor: the next docs-accuracy pass scoped to this page, which the drift advisory on this PR already lists.

Generated by Claude Code

…ma.manifestId

`packages/cli/src/commands/package/publish.ts` carried its own
`MANIFEST_ID_RE` and tested `--manifest-id` (and the derived id) against it,
while the contract this repo declares for the column it publishes into is
`PackageSchema.manifestId` in `packages/spec/src/cloud/package.zod.ts`. The
local copy was looser on every axis, so the preflight admitted what the
control plane refuses.

- Delete `MANIFEST_ID_RE`. Both paths — the explicit `--manifest-id` /
  `objectstack.manifest.json` check and the derive path in
  `deriveManifestId` — now parse through the imported schema.
- The derive path's extra `explicit.includes('.')` condition is dropped: the
  schema subsumes it (its pattern needs at least two segments). That
  condition is why the two paths disagreed with each other as well as with
  the declaration.
- The refusal text is quoted from the schema's own `invalid_format` issue and
  its `.describe()`, so it can no longer state a contract that does not exist.
- A derived id the schema rejects (`slugify` has no letter-first rule, so an
  app named `2024 App` derives `local.2024-app`) is refused before any network
  call, naming where the id came from and how to set one. It is deliberately
  not normalised: `manifestId` is immutable once published.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
@github-actions github-actions Bot added the size/m label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 11 documentable anchor(s).

11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/declarative-endpoints.mdx (via os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/concepts/metadata-lifecycle.mdx (via os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/deployment/cli.mdx (via com.acme.crm (literal, a string literal in PackagePublish; a string literal on a changed line), os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/deployment/index.mdx (via com.acme.crm (literal, a string literal in PackagePublish; a string literal on a changed line), os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/deployment/publish-and-preview.mdx (via com.acme.crm (literal, a string literal in PackagePublish; a string literal on a changed line), os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/kernel/contracts/metadata-service.mdx (via com.acme.crm (literal, a string literal in PackagePublish; a string literal on a changed line))
  • content/docs/protocol/kernel/index.mdx (via os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/protocol/kernel/lifecycle.mdx (via os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/protocol/kernel/metadata-service.mdx (via os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/protocol/objectql/index.mdx (via os package publish (command, read off packages/cli/src/commands/package/publish.ts))
  • content/docs/protocol/objectql/schema.mdx (via os package publish (command, read off packages/cli/src/commands/package/publish.ts))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v9.mdx (via os package publish (command, read off packages/cli/src/commands/package/publish.ts))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 142c01c88ee1be4c24e20f7b803a511a00c992fbpackageMentionDocs.

Which tree this was computed on

This run read content/docs from a6204a5d87b55f0755380c58f6f4d7745b1ce704 — the merge of head 8756045d1a4b82cc711e09d105d2443abace1af7 into base 142c01c88ee1be4c24e20f7b803a511a00c992fb, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a6204a5d87b55f0755380c58f6f4d7745b1ce704 && git checkout a6204a5d87b55f0755380c58f6f4d7745b1ce704
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 142c01c88ee1be4c24e20f7b803a511a00c992fb 8756045d1a4b82cc711e09d105d2443abace1af7 && git checkout -B drift-repro 142c01c88ee1be4c24e20f7b803a511a00c992fb && git merge --no-ff 8756045d1a4b82cc711e09d105d2443abace1af7

node scripts/docs-audit/affected-docs.mjs --json 142c01c88ee1be4c24e20f7b803a511a00c992fb

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 142c01c88ee1be4c24e20f7b803a511a00c992fb → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants